ios: support additional options pass from js #188

zxcpoiu · 2020-04-24T06:56:28Z

Support addition options can pass from js method

`updateDisplay`

original:

updateDisplay = (uuid, displayName, handle)

new:

updateDisplay = (uuid, displayName, handle, options = null)
where the optional options could be:

{
  hasVideo: true,
  supportsHolding: true,
  supportsDTMF: true,
  supportsGrouping: true,
  supportsUngrouping: true,
}

`displayIncomingCall`

original:

displayIncomingCall = (uuid, handle, localizedCallerName, handleType = 'number', hasVideo = false)

new:

displayIncomingCall = (uuid, handle, localizedCallerName, handleType = 'number', hasVideo = false, options = null)
where the optional options could be:

{
  supportsHolding: true,
  supportsDTMF: true,
  supportsGrouping: true,
  supportsUngrouping: true,
}

Note for API compatibility:

Originally, displayIncomingCallalready has hasVideo argument but updateDisplay are not. For api compatibility, I did NOT move displayIncomingCall.hasVideo inside displayIncomingCall.options

In the earlier time, we've exposed native reportNewIncomingCall and introduced an overloading function for the new payloadargument for api compatibility:

Earlier Original (as simple api):

[RNCallKeep reportNewIncomingCall:uuid handle:handle handleType:@"generic" hasVideo:false localizedCallerName:callerName fromPushKit: YES];

and

Earlier New (as full api):

[RNCallKeep reportNewIncomingCall:uuid handle:handle handleType:@"generic" hasVideo:false localizedCallerName:callerName fromPushKit: YES payload:extra];

We can't have too much overloading functions, so I keep the original one, and extend the new overloading function, so now is:

This PR Original (as simple api):

[RNCallKeep reportNewIncomingCall:uuid handle:handle handleType:@"generic" hasVideo:false localizedCallerName:callerName fromPushKit: YES];

and

This PR New (as full api):

[RNCallKeep reportNewIncomingCall:uuid handle:handle handleType:@"generic" hasVideo:false localizedCallerName:callerName supportsHolding: YES supportsDTMF: YES supportsGrouping: YES supportsUngrouping: YES fromPushKit: YES payload: payload];

TL;DR

For user using native RNCallKeep reportNewIncomingCall with payload argument should change to [RNCallKeep reportNewIncomingCall:uuid handle:handle handleType:@"generic" hasVideo:false localizedCallerName:callerName supportsHolding: YES supportsDTMF: YES supportsGrouping: YES supportsUngrouping: YES fromPushKit: YES payload: payload];

…-native-callkeep

sboily · 2020-04-28T02:19:50Z

Hello, does this features are possible on android? I haven't checked.

sboily · 2020-04-28T02:30:25Z

I checked the connection service API and looks like there is nothing to control that. options looks like generic, but what we want to do with android with this options? Does options need to be renamed to be more ios specific? Can you add the documentation on the README?

zxcpoiu · 2020-04-28T07:52:53Z

@sboily

Yes, on android, currently is no-op to the options.

About the naming, I just left it as it's original name of iOS because in the original api displayIncomingCall, there is an arguments called hasVideo which is no-op on android as well.
see here

I could change the name in the options with ios prefix indicating it's an ios specific option, like: iosHasVideo, but it looks like uncoordinated with the original hasVideo arg in displayIncomingCall described above.

Maybe we could change to like this?

let options = {
  ios: {
    hasVideo: true,
    supportsHolding: true,
    supportsDTMF: true,
    supportsGrouping: true,
    supportsUngrouping: true,
  }
}

Because this PR related to api definition and slightly breaking change, so I will add document once we are all agreed the final api and this PR then :)

danjenkins · 2020-04-28T08:25:59Z

I think thats valid @zxcpoiu - its really difficult to merge everything into a combined API, I have partial video support going in Android but it really needs more work before I do a PR but we could just have an android key in the object - android doesnt support "supportsDTMF" for example... "supportsHolding" is done by capability flags passed into various things in connection service

sboily · 2020-04-28T13:16:42Z

Hello, i prefer this approach @zxcpoiu.

…comingCall. Which is usefull if you want to tell the pushkit you handled the VoIP notification only after the call was successfully reported.

zxcpoiu · 2020-04-29T08:15:02Z

@danjenkins @sboily

To confirm, we are saying that we prefer below approach, right?

let options = {
  ios: {
    hasVideo: true,
    supportsHolding: true,
    supportsDTMF: true,
    supportsGrouping: true,
    supportsUngrouping: true,
  }
}

danjenkins · 2020-04-29T08:16:39Z

I am. We just can't hide the intrinsic differences between the platform's completely - android actually has a load more functionality not yet exposed!

sboily · 2020-04-29T10:09:01Z

Yes, i also confirm, thank you.

manuquentin · 2020-04-29T11:48:32Z

@zxcpoiu can you rebase on master please ?

…er, so it is not a breaking change.

zxcpoiu · 2020-04-30T08:37:04Z

Done!

Changed to platform specific options
Add documents

If you want the api to be unified, I'm happy to move hasVideo arg of displayIncomingCall into options as well, but this introduced breaking changes to every user which invoke displayIncomingCall from js.

Let me know if anything need to change.

…ommits this is my first time doing this

…-native Separate react native part from rest of Android code

manuquentin · 2020-06-04T15:47:09Z

@zxcpoiu can you rebase it please ? So we can merge it.

…ack_to_foreground_killed [Android] BackToForeground when app is killed.

manuquentin · 2020-06-30T12:44:19Z

@zxcpoiu can you rebase your PR ? Thanks !

answerIncomingCall is missing from the index.d.ts file

Update index.d.ts

…evert_wakeup_app Android: Revert wakeUpApplication to use react-native class

zxcpoiu · 2020-07-13T06:37:47Z

sorry for the delay, rebased.
But now seems guthub have some issue, the PR did not refreshed yet :(

zxcpoiu · 2020-07-13T06:55:51Z

But I noticed this PR needs to be discussed about the overloading api reportNewIncomingCall

Due to the addition of withCompletionHandler, now we have 3 overloading functions for backward compatibility:

Main functioon

main function should contains all arguments

https://github.com/react-native-webrtc/react-native-callkeep/blob/master/ios/RNCallKeep/RNCallKeep.m#L369

+ (void)reportNewIncomingCall:(NSString *)uuidString
                       handle:(NSString *)handle
                   handleType:(NSString *)handleType
                     hasVideo:(BOOL)hasVideo
          localizedCallerName:(NSString * _Nullable)localizedCallerName
                  fromPushKit:(BOOL)fromPushKit
                      payload:(NSDictionary * _Nullable)payload
        withCompletionHandler:(void (^_Nullable)(void))completion
{

Overloading 1

When we was adding payload, we introduced overloading function for user who is not using payload

https://github.com/react-native-webrtc/react-native-callkeep/blob/master/ios/RNCallKeep/RNCallKeep.m#L416

+ (void)reportNewIncomingCall:(NSString *)uuidString
                       handle:(NSString *)handle
                   handleType:(NSString *)handleType
                     hasVideo:(BOOL)hasVideo
          localizedCallerName:(NSString * _Nullable)localizedCallerName
                  fromPushKit:(BOOL)fromPushKit
{

Overloading 2

When we was adding withCompletionHandler, we introduced overloading function for user who is not using withCompletionHandler ( but using payload )

https://github.com/react-native-webrtc/react-native-callkeep/blob/master/ios/RNCallKeep/RNCallKeep.m#L358

+ (void)reportNewIncomingCall:(NSString *)uuidString
                       handle:(NSString *)handle
                   handleType:(NSString *)handleType
                     hasVideo:(BOOL)hasVideo
          localizedCallerName:(NSString * _Nullable)localizedCallerName
                  fromPushKit:(BOOL)fromPushKit
                      payload:(NSDictionary * _Nullable)payload
{

zxcpoiu · 2020-07-13T07:05:13Z

Main function should have all arguments for sure:

+ (void)reportNewIncomingCall:(NSString *)uuidString
                       handle:(NSString *)handle
                   handleType:(NSString *)handleType
                     hasVideo:(BOOL)hasVideo
          localizedCallerName:(NSString * _Nullable)localizedCallerName
              supportsHolding:(BOOL)supportsHolding
                 supportsDTMF:(BOOL)supportsDTMF
             supportsGrouping:(BOOL)supportsGrouping
           supportsUngrouping:(BOOL)supportsUngrouping
                  fromPushKit:(BOOL)fromPushKit
                      payload:(NSDictionary * _Nullable)payload
        withCompletionHandler:(void (^_Nullable)(void))completion
{

So now we have to decide:

what kind and how many overloading functions we should keep for api backward compatibility?

sboily · 2020-07-14T01:28:49Z

Hello @zxcpoiu, yes ! you right we need to cleanup a little that. We could probably release a new major version with some new features like #237 and #188 and removing some old backward compatibilities if there is no strong objection here. I think with these two PRs we have some interesting works to have video capabilities in callkeep for android and iOS.
Can you make a new tentative for updating the PR please?
Thank you!

zxcpoiu · 2020-07-14T09:50:51Z

The commit history is getting funky on github, not sure why. force push and delete branch does't fix it.
I continued at #238

LinusU and others added 9 commits December 9, 2019 17:44

Remove dependency on RN* part from other part of Android code

9d8f656

Remove unused imports

b4213dc

Remove dependency on com.facebook.* from non-RN part of Android code

4ee21f7

Cleanup constant definitions

37b543b

checkIfActiveCall

460a14f

Minor change

a4a33e1

Updated with requested changes

5286fee

Merge branch 'master' of https://github.com/react-native-webrtc/react…

14232a2

…-native-callkeep

Forgot to update RNCallKeep.h to reflect rename

9df84d8

lrorthmann added 2 commits April 28, 2020 14:06

Adding the possibility to add a completionHandler for the reportNewIn…

e849f27

…comingCall. Which is usefull if you want to tell the pushkit you handled the VoIP notification only after the call was successfully reported.

Updating README.md

96e8133

lrorthmann and others added 2 commits April 29, 2020 15:11

Changed to overload reportNewIncomingCall method with the new paramet…

1b98744

…er, so it is not a breaking change.

Exposed to JS

f043034

zxcpoiu force-pushed the support_options branch from bfac182 to 24a1e9c Compare April 30, 2020 08:30

ajnozari added 3 commits April 30, 2020 04:30

Updated TS

dde9e3f

Updated TS .... again

46fe05e

More TS, new to TS in general

dd6e0b5

ajnozari added 3 commits April 30, 2020 04:40

Still new to new additions

15e9259

Updated to properly expose to RCT EXPORT METHOD, please forgive the c…

00ef852

…ommits this is my first time doing this

Updated to properly expose to RCT EXPORT METHOD, please forgive the c…

5ee1e4a

…ommits this is my first time doing this

Merge pull request react-native-webrtc#137 from LinusU/separate-react…

59b2526

…-native Separate react native part from rest of Android code

manuquentin added 6 commits June 19, 2020 16:53

3.0.14

749b014

[Android] backToForeground: allow app to start when killed

1e8987a

Document backToForeground

2a31f3a

Merge pull request react-native-webrtc#223 from react-native-webrtc/b…

bedfc23

…ack_to_foreground_killed [Android] BackToForeground when app is killed.

3.0.15

3fe4da5

Android: Revert wakeUpApplication to use react-native class

a03628c

CaptainJeff and others added 7 commits June 30, 2020 08:51

Update index.d.ts

a67421c

answerIncomingCall is missing from the index.d.ts file

Merge pull request react-native-webrtc#235 from CaptainJeff/patch-1

082b0d0

Update index.d.ts

Merge pull request react-native-webrtc#234 from react-native-webrtc/r…

4cdf17f

…evert_wakeup_app Android: Revert wakeUpApplication to use react-native class

ios: implement support options for disaply incoming call

65ac242

ios: implemet support options for updateDisplay

f35ce84

ios: platform specific options for displayIncomingCall and updateDisplay

0137e2e

readme: add options arg for displayIncomingCall and updateDisplay

ee625b5

ios: clean/remove reportNewIncomingCall overloading api

0af9f9e

zxcpoiu closed this Jul 14, 2020

zxcpoiu deleted the support_options branch July 14, 2020 09:43

zxcpoiu restored the support_options branch July 14, 2020 09:43

zxcpoiu mentioned this pull request Jul 14, 2020

ios: support additional options pass from js #238

Merged

zxcpoiu deleted the support_options branch July 14, 2020 09:46

zxcpoiu restored the support_options branch July 14, 2020 09:46

zxcpoiu deleted the support_options branch July 14, 2020 09:46

zxcpoiu restored the support_options branch July 14, 2020 09:47

ios: support additional options pass from js #188

ios: support additional options pass from js #188

Uh oh!

Conversation

zxcpoiu commented Apr 24, 2020

updateDisplay

original:

new:

displayIncomingCall

original:

new:

Note for API compatibility:

Earlier Original (as simple api):

Earlier New (as full api):

This PR Original (as simple api):

This PR New (as full api):

TL;DR

Uh oh!

sboily commented Apr 28, 2020

Uh oh!

sboily commented Apr 28, 2020

Uh oh!

zxcpoiu commented Apr 28, 2020

Uh oh!

danjenkins commented Apr 28, 2020

Uh oh!

sboily commented Apr 28, 2020

Uh oh!

zxcpoiu commented Apr 29, 2020

Uh oh!

danjenkins commented Apr 29, 2020

Uh oh!

sboily commented Apr 29, 2020

Uh oh!

manuquentin commented Apr 29, 2020

Uh oh!

zxcpoiu commented Apr 30, 2020

Uh oh!

manuquentin commented Jun 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

manuquentin commented Jun 30, 2020

Uh oh!

zxcpoiu commented Jul 13, 2020

Uh oh!

zxcpoiu commented Jul 13, 2020

Main functioon

Overloading 1

Overloading 2

Uh oh!

zxcpoiu commented Jul 13, 2020

Main function should have all arguments for sure:

So now we have to decide:

Uh oh!

sboily commented Jul 14, 2020

Uh oh!

zxcpoiu commented Jul 14, 2020

Uh oh!

Uh oh!

`updateDisplay`

`displayIncomingCall`

manuquentin commented Jun 4, 2020 •

edited

Loading